refactor(settings): retire the DB->.env export; decouple the standalone CLI (ENG-1295) - #280
Draft
pnewsam wants to merge 13 commits into
Draft
refactor(settings): retire the DB->.env export; decouple the standalone CLI (ENG-1295)#280pnewsam wants to merge 13 commits into
pnewsam wants to merge 13 commits into
Conversation
Make cowork-server mirror its settings out to the standalone `anton` CLI's .env after every write, so the DB is the source of truth and .env is a derived export rather than a competing one. Runs only for local (single-user desktop) tenancy; a multi-tenant cloud pod writes no .env. Re-derives the aliased keys (SETTING_ENV_ALIASES) from the DB at the shared post-commit seam, decrypting secrets, using the CLI's dash-form provider values, and merge-preserving every unmanaged line (auth token, CLI-only model pins per ENG-739, comments). Atomic write, 0o600, skipped when unchanged, and never raises into the settings write. The startup .env->DB migration passes export_env=False so it can't rewrite the file it seeds from. Phase A of ENG-1127. Phase B (removing the client's own .env writes + login replay) is a follow-up, gated on this landing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment/docstring-only cleanup (~half the added comment lines); no code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ary.py (ENG-1127) Move all .env-boundary knowledge out of user_settings.py into one module owning both directions: the alias maps (SETTING_ENV_ALIASES / ENV_ALIAS_TO_SETTING), normalize_provider_value, the inbound env_to_db_updates (extracted from the migration's inline map+normalize loops), the outbound db_to_env (was build_env_export), and the file I/O (merge_env_lines / atomic_write_env). user_settings.py is now purely the DB model; Provider.ui_value stays on the enum (used beyond .env). env_export.py is absorbed and removed. Pure reorg — no behavior change (env_to_db_updates reproduces the old inline loops exactly). Full server suite green apart from the pre-existing unrelated test_comments_layer::test_serve_injects_only_with_flag (fails on staging too). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…xport (ENG-1127 review) Two code-review findings on the .env exporter: - backfill_minds_url wrote minds_url directly and only invalidated the cache, so the DB moved to the canonical MindsHub host while the CLI's .env kept the dead mdb.ai one. Route it through the exporting hook (svc._after_write) so both stores update. Regression test asserts both. - the export's read/merge/write wasn't serialized, so two concurrent settings writes could lost-update .env. Guard it with a module-level lock, re-reading the DB inside the lock so the last exporter installs the latest committed state. Regression test asserts the critical section admits one writer at a time. (Cross-process competition with the client's own .env writes is transient and self-heals via the DB re-derive; Phase B removes the client writer.) Full server suite green (702 passed) apart from the pre-existing unrelated test_comments_layer failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s (ENG-1127) The CLI (or a version-skewed server) holding ~/.cowork/.env open makes the final os.replace raise a transient PermissionError on Windows — the exact EPERM that wedged onboarding on the client before it grew a retry (ENG-1209). ENG-1127 moves .env writing to the server (Phase B removes the client's hardened writer), so that hardening has to live here too or the bug re-opens on the server side. atomic_write_env now retries the rename on transient lock errnos (EPERM/EACCES/EBUSY/ENOTEMPTY) with a widening backoff mirroring the client's retryOnTransientLock, and sweeps stale orphaned .env.*.tmp files (they hold the full plaintext key) while sparing a concurrent writer's fresh temp. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ENG-1127 review) A CR/LF in an exported value (e.g. a poisoned minds_url like "https://x\nDATABASE_URI=…") would terminate the ANTON_* assignment and turn the remainder into a second, unmanaged line that survives every later merge and is consumed on the next CLI/server start. The exported fields never legitimately contain a newline, so db_to_env now drops such a value (best-effort, logged) and merge_env_lines skips it as a serialization invariant. Adds round-trip/injection regression tests, incl. an end-to-end save that confirms the settings layer does not itself block newlines — so this guard is load-bearing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, gemini translated (ENG-1127 review) The DB->.env export was a naive field dump, which produced CLI configs the pinned anton (c51afe87) cannot run (review findings 1 & 2): - A provider was exported without its model (models are excluded from the alias map, ENG-739), so switching the DB provider left a stale, mismatched ANTON_*_MODEL line — e.g. provider=openai against a leftover Claude model. - provider=gemini was written literally, but anton has no first-class gemini provider (from_settings raises "Unknown planning provider: gemini"); it runs Gemini as openai-compatible + Google's base URL + the key in the OpenAI slot. ANTON_GEMINI_API_KEY / ANTON_OPENAI_API_KEY_CUSTOM are fields anton doesn't read, so exporting them was a silent no-op. db_to_env now renders the provider/model/key/base cluster in anton's on-disk vocabulary using the SAME resolution the server's own build_llm_client applies (resolved_*_provider/model + provider_base_url + provider_api_key): each role's provider is written WITH its resolved model (the pair is always valid), gemini is translated to openai-compatible + GEMINI_BASE_URL + the OpenAI key slot, and a role whose resolved provider has no key exports nothing. The per-role model vars are now MANAGED so a stale/orphaned model line is reconciled away. Verified all three provider shapes round-trip through the pinned anton's from_settings. Non-provider settings (memory flags, publish URL) keep the straight present-gated alias. Tests updated for the resolved+paired behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…esent (ENG-1127 review) The product runs mixed per-role providers via independent per-role LLM clients (build_llm_client derives each role's base URL separately), but the standalone anton CLI has a SINGLE global ANTON_OPENAI_API_KEY/ANTON_OPENAI_BASE_URL pair handed to both its openai and openai-compatible factories, and its MindsHub derivation only fires when that OpenAI key is unset. So planning=OpenAI + coding=Gemini would export OpenAI's key against Google's base (misrouting both roles), and MindsHub + OpenAI would break the Minds derivation. db_to_env now checks representability before emitting the provider cluster: every OpenAI-slot role must agree on the same (key, base), and minds-cloud must not coexist with an explicit OpenAI-slot role. A non-representable config exports NO provider/model/creds (logged) — leaving the CLI on its own config rather than a silently-misrouting one. Representable mixes that use independent slots (e.g. anthropic + gemini) still export both roles. Adds mixed-provider regression cases for both the rejected and the allowed shapes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeQL's incomplete-url-substring-sanitization flags any `.startswith("https://…")`
prefix check on a URL host. These are test assertions, but the fix is also
cleaner: compare the exported base URL for exact equality against the canonical
GEMINI_BASE_URL constant instead of a prefix substring.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pe a valid .env on unrepresentable saves (ENG-1127 review) Two Major edge cases from review: 1. Router-only MindsHub. The export trusted Anton's model_post_init to derive the OpenAI slot from the minds_* creds, but pinned Anton (c51afe87) only derives when the PLANNING or CODING provider is openai-compatible — never a router-only Minds role. So planning/coding=Anthropic + router=MindsHub built the router with no key/base. Fix: fold minds-cloud into the shared-OpenAI-slot model and export ANTON_OPENAI_API_KEY / ANTON_OPENAI_BASE_URL EXPLICITLY (minds key + minds_chat_base_url), keeping the minds_* slots too. Verified the router-only shape round-trips through the pinned Anton's from_settings. This also simplifies representability to a single rule (all OpenAI-slot roles must agree). 2. Unrepresentable save wiped a valid .env. Returning no provider cluster let merge_env_lines delete every managed provider/model/cred line, wiping a previously-valid standalone CLI config on the next save. Fix: env_reconcile_vars narrows the merge's drop-set to the flag vars when the config is unrepresentable, PRESERVING the existing cluster; a genuinely cleared config (no keys) is still representable, so logout still wipes creds. merge_env_lines also drops any var it is about to write, preventing duplicates. Added an end-to-end test starting from a pre-populated .env plus the router-only round-trip test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ess clusters; pair router with its own model (ENG-1127 review) Three Major findings: 1. /settings/raw synced the WHOLE merged .env back into the DB, so a preserved or translated cluster (a stale minds-cloud line, or a gemini role exported as openai-compatible) would overwrite the authoritative DB choice on the next unrelated raw write (OAuth/token refresh). It now syncs ONLY the recognised vars present in the incoming request — the exact root cause the client already worked around by moving sign-in off /raw (ENG-739). Models still never sync. 2. An openai-compatible/minds provider with no base URL passed representability, and the per-field CR/LF filter could drop a base while keeping provider+key — Anton then defaults to https://api.openai.com/v1/ and leaks a Minds/custom key to OpenAI. The provider cluster is now built and validated ATOMICALLY (_provider_cluster): a base-requiring provider with no base, or any non-dotenv-safe value, drops the WHOLE cluster (and preserves the existing .env), never a lone field. env_reconcile_vars keys off the same decision so "wrote nothing" and "preserve" can't disagree. 3. apply_model_defaults derived an absent router_model from coding_provider, so coding=Anthropic + router=OpenAI exported ANTON_ROUTER_PROVIDER=openai with a Claude model. It now derives from router_provider, matching planning/coding. Adds regressions: sync-incoming-only (stale-cluster) raw test, missing-base atomic drop, router provider/model pairing (round-tripped through pinned Anton), and atomic-drop injection behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…G-1127 review) The legacy POST /settings/raw wrote ~/.cowork/.env with a bare write_text + chmod, bypassing everything the DB->.env export hardened: temp-file+os.replace atomicity, the CR/LF dotenv-safety guard, the transient-Windows-lock retry, and the export lock. After Phase B the client no longer calls /raw, so it's dormant- legacy and loopback-only — but it remained a second, weaker .env writer that could race the managed export. Route it through atomic_write_env and skip any newline-bearing var so this path can't smuggle a second line either. Self-review of PR #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-1295) Remove the DB->.env export shim (added for the standalone anton CLI) and isolate the embedded harness base, so the DB is the sole server-side settings store and the standalone CLI owns its own ~/.anton/.env. - env_boundary.py: keep the inbound .env->DB conversion (used by the one-time boot migration); delete the outbound export (db_to_env, provider-cluster / representability logic, merge_env_lines, atomic_write_env + Windows lock-retry helpers). 494 -> ~54 lines. - services/settings.py: drop _export_env_for_cli, the export_env param, and the export lock; _after_write keeps the settings-cache invalidation. - api/v1/endpoints/settings.py: remove the GET/POST /settings/raw handlers (no client caller remains after cowork #524). - harnesses/anton_harness/harness.py: build AntonSettings(_env_file=None) so a user's standalone ~/.anton/.env can't bleed into embedded Cowork sessions. - migrations.py: drop the export_env kwarg; refresh stale docstrings. - tests: delete test_env_boundary.py (export) and test_settings_raw.py (/raw); trim the /raw case from test_settings_local_guard.py. Full suite: 753 passed, 0 regressions (2 pre-existing unrelated failures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No PR environment for this pull requestAdd the Updated on every push to this PR. |
This was referenced Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Refashions #253 to the sustainable end-state Zoran asked for in review: instead of adding a DB→
.envexport shim to feed the standaloneantonCLI, the DB becomes the sole server-side settings store and the CLI is decoupled onto its own~/.anton/.env.env_boundary.py— kept the inbound.env→DB conversion (used by the one-time boot migration); deleted the outbound export (db_to_env, provider-cluster/representability logic,merge_env_lines,atomic_write_env+ Windows lock-retry). 494 → ~54 lines.services/settings.py— dropped_export_env_for_cli, theexport_envparam, and the export lock._after_writekeeps the settings-cache invalidation.api/v1/endpoints/settings.py— removed the GET/POST/settings/rawhandlers; no client caller remains after cowork #524.harnesses/anton_harness/harness.py— buildAntonSettings(_env_file=None)so a user's standalone~/.anton/.envcan't bleed non-overlaid fields into embedded Cowork sessions (the DB overlay stays authoritative).migrations.py— dropped theexport_envkwarg; refreshed stale docstrings.test_env_boundary.py(all export) andtest_settings_raw.py(all/raw); trimmed the/rawcase fromtest_settings_local_guard.py.Net vs
staging: +134 / −342 — a net deletion, because the export shim was ~70% of #253.Why
The mirror only shadowed config the CLI already owns and was the last file-based settings path behind the desync bug class (ENG-848 / ENG-922 / ENG-918). Decoupling deletes it outright rather than landing it to remove later. The standalone CLI must run without a local server, so it reads its own
~/.anton/.env; when anton runs embedded, cowork-server keeps overlaying the DB.Relationship to existing PRs
~/.cowork/.envfrom the CLI chain).Testing
Full suite: 753 passed, 0 regressions (2 pre-existing, unrelated artifact/comment failures). Harness isolation verified empirically.
Draft. Tracked in ENG-1295.
🤖 Generated with Claude Code